-
Notifications
You must be signed in to change notification settings - Fork 295
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: Used dumped state instead of fork #1399
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you turn the e2e test back on in config yml
// anvil --fork-url https://mainnet.infura.io/v3/9928b52099854248b3a096be07a6b23c --fork-block-number 17514288 --chain-id 31337 | ||
// For CI, this is configured in `run_tests.sh` and `docker-compose.yml` | ||
|
||
const dumpedState = 'src/dumps/uniswap_state'; | ||
const EXPECTED_FORKED_BLOCK = 0; //17514288; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit - add a comment on how dumped state doesn't change block number but it works nevertheless
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm, however id prefer dumps to be in a fixtures folder
6e4bbe4
to
4d04a4f
Compare
Will create a separate pr that fixed fixtures. There are also a couple of other files that would make sense to put in so would pollute this pr. |
* Updated fork block (#1396) * Updated fork block * Set redeploy flag * Redploy dev net (#1398) * Force redeploy (#1399) Co-authored-by: joss-aztec <[email protected]>
Fixes #1367
Use a chain dump instead of forking for uniswap tests.
The sandbox is still using the fork.
Checklist:
Remove the checklist to signal you've completed it. Enable auto-merge if the PR is ready to merge.